Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent variable_init from retuning an empty array. #37

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

mikeytown2
Copy link

Patch does a couple of things

  • If the recursion_depth hit the limit, assume we aren't going to get it from the cache or the lock will be released any time soon. Give up and get variables from the database.
  • If the db result is FALSE run _db_error_page(). This prevents the cache from getting polluted with bad (empty) data.
  • Various white space fixes.

@mmurshed
Copy link

mmurshed commented Apr 3, 2012

I see only one new line and a bunch of white spaces. :)

@mikeytown2
Copy link
Author

There are 41 additions and 21 deletions; that means we have 20 new lines. About 2/3 of the new lines are comments, but there is a fair share of new code. The new logic that is inserted in 2 different places deals with this chunk of code. Click on the diff button above next to Discussion & Commits to see the diff. Or to see the issue on d.o go here http://drupal.org/node/561990#comment-5803696

  // If the recursion_depth hit the limit, assume we aren't going to get it
  // from the cache or the lock will be released any time soon. Give up and
  // get variables from the database.
  $result = db_query('SELECT * FROM {variable}');
  // Exit here if the database went away. Do not want to pollute the cache
  // with bad data. This request isn't going to end well.
  if ($result === FALSE) {
    // This function calls exit.
    _db_error_page();
  }
  while ($variable = db_fetch_object($result)) {
    $variables[$variable->name] = unserialize($variable->value);
  }

@mmurshed
Copy link

mmurshed commented Apr 3, 2012

Sorry, I did not want to be rude. The white spaces just caught my eyes. I really appreciate the work you do for the Drupal community through many great modules.

@mikeytown2
Copy link
Author

White space fixes are a result of the editor I have (kate) :) It removes trailing white space at the end of a line. http://drupal.org/coding-standards#indenting

rjbrown99 added a commit to rjbrown99/6 that referenced this pull request May 1, 2012
mikeytown2 added 24 commits March 19, 2013 14:42
… unlink() No such file or directory in file_delete()
Unknown error: Function split() is deprecated in _filter_autop()
…ails. Add in the ability to handle this gracefully (don't always call _db_error_page).
@dimduj
Copy link

dimduj commented Oct 10, 2013

Hello,

Thanks for the patch MickeyTown, it is really usefull on our production server.

I've juste a small question about releasing the lock when we detect a FALSE result.

Shouldn't we do something like that (if false then release lock):

if (defined('MAINTENANCE_MODE') || lock_acquire('variable_cache_regenerate')) {
  $result = db_query('SELECT * FROM {variable}');
  // Exit here if the database went away. Do not want to pollute the cache
  // with bad data. This request isn't going to end well anyway; end it
  // eairly.
  if ($result === FALSE) {
    // This function calls exit.
    lock_release('variable_cache_regenerate');
    _db_error_page();
  }
  while ($variable = db_fetch_object($result)) {
    $variables[$variable->name] = unserialize($variable->value);
  }


  cache_set('variables', $variables);
  if (!defined('MAINTENANCE_MODE')) {
    lock_release('variable_cache_regenerate');
  }
}

Thanks for any advice.

regards,

Dimitri

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants